Add tests #16
Merged
Add tests #16
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
The tests require that #14 is merged in order to pass, but otherwise adds 100% code coverage. |
kandersolar
reviewed
Feb 28, 2022
twoaxistracking/tests/test_layout.py
Outdated
| # Test calculation of min_tracker_spacing for a polygon | ||
| collector_geometry = geometry.Polygon([(-1, -1), (3, 2), (4, 4), (1, 2), (-1, -1)]) | ||
| min_tracker_spacing = layout._calculate_min_tracker_spacing(collector_geometry) | ||
| assert min_tracker_spacing == 2 * np.sqrt(4**2 + 4**2) |
Member
There was a problem hiding this comment.
Direct equality checking of floats is a bit risky, especially in the age of SIMD-enabled numpy. Consider changing to an np.testing assertion or pytest.approx here and above to have a bit of tolerance
Member
Author
There was a problem hiding this comment.
Is it ok with direct checking of zero and integers? For example:
assert shaded_fraction == 0
Member
Author
There was a problem hiding this comment.
Also, is np.isclose sufficient for the above test?
Member
There was a problem hiding this comment.
Good point, I'll amend my previous statement to say "...a bit risky (expect for some special floating point values like zero)". Yeah I'd say using == is the best option for checking 0 or 1.
In this case I'd probably still use a dedicated assertion function over assert function_that_returns_bool() though. assert bool hardly has any info to work with, so it can't tell you why the assert failed (although pytest has some magic that lets it inspect the original expression sometimes), while a dedicated assertion function has access to everything and can make a nice message.
Co-authored-by: Kevin Anderson <[email protected]>
Co-authored-by: Kevin Anderson <[email protected]>
Co-authored-by: Kevin Anderson <[email protected]>
Member
Author
|
Due to a mix-up in commits, this branch was reset to main and any commits prior to this comment should be ignored. |
kandersolar
reviewed
Mar 1, 2022
Member
kandersolar
left a comment
kandersolar
left a comment
There was a problem hiding this comment.
looks good to me, one minor comment about fixtures
kandersolar
approved these changes
Mar 2, 2022
Member
kandersolar
left a comment
kandersolar
left a comment
There was a problem hiding this comment.
Looks great @AdamRJensen :)
AdamRJensen
added a commit
that referenced
this pull request
Mar 6, 2022
commit f435e1c Author: Adam R. Jensen <[email protected]> Date: Wed Mar 2 15:52:15 2022 +0100 Add tests (#16) * Create twoaxistrackerfield.py * Add TwoAxisTrackerField to __init__.py and documentation.rst * Add _calculate_l_min to layout * Fix linter error * Update twoaxistrackerfield.py * Reorder layout ValueError warnings * Fix typo * Remove layout_type from generate_field_layout function * Add layout_type code to TwoAxisTrackerField * L_min -> min_tracker_spacing * Add whatsnew entry * Fix linting errors * Update twoaxistrackerfield.py * Correct horizon shading * Update twoaxistracking/twoaxistrackerfield.py Co-authored-by: Kevin Anderson <[email protected]> * Update twoaxistracking/twoaxistrackerfield.py Co-authored-by: Kevin Anderson <[email protected]> * Update twoaxistracking/twoaxistrackerfield.py Co-authored-by: Kevin Anderson <[email protected]> * Return scalar when scalar is passed * Correct use of np.isscalar * Update documentation * Update whatsnew * Rework intro_tutorial.ipynb * Update README.md * Create test_layout.py * Update test_layout.py * Use test folder in package folder * Delete test_placeholder.py * Add test files * Fix linting errors * Update scalar test * Add pandas to test in setup.cfg * Revert "package metadata and doc build updates (#17)" This reverts commit e9b5810. * Undo commits * Revert "Undo commits" This reverts commit 147ba2a. * Add test files * Switch to using np.testing.assert_allclose * Add test for sloped field layout * Move test_twoaxistracker to twoaxistracker PR * Add conftest.py to avoid duplicate fixtures * Remove imports of fixtures from conftest * Remove unnecessary imports * Increase test tolerance for test_field_slope * Add multi-polygon active area test * Add empty __init__.py * Use "from .conftest" as Kevin said.. * Base square_field_layout_slope fixture off square_field_layout * Fix linter error * Remove backslash in conftest.py Co-authored-by: Kevin Anderson <[email protected]>
AdamRJensen
added a commit
that referenced
this pull request
Mar 10, 2022
* Squashed commit of the following: commit f435e1c Author: Adam R. Jensen <[email protected]> Date: Wed Mar 2 15:52:15 2022 +0100 Add tests (#16) * Create twoaxistrackerfield.py * Add TwoAxisTrackerField to __init__.py and documentation.rst * Add _calculate_l_min to layout * Fix linter error * Update twoaxistrackerfield.py * Reorder layout ValueError warnings * Fix typo * Remove layout_type from generate_field_layout function * Add layout_type code to TwoAxisTrackerField * L_min -> min_tracker_spacing * Add whatsnew entry * Fix linting errors * Update twoaxistrackerfield.py * Correct horizon shading * Update twoaxistracking/twoaxistrackerfield.py Co-authored-by: Kevin Anderson <[email protected]> * Update twoaxistracking/twoaxistrackerfield.py Co-authored-by: Kevin Anderson <[email protected]> * Update twoaxistracking/twoaxistrackerfield.py Co-authored-by: Kevin Anderson <[email protected]> * Return scalar when scalar is passed * Correct use of np.isscalar * Update documentation * Update whatsnew * Rework intro_tutorial.ipynb * Update README.md * Create test_layout.py * Update test_layout.py * Use test folder in package folder * Delete test_placeholder.py * Add test files * Fix linting errors * Update scalar test * Add pandas to test in setup.cfg * Revert "package metadata and doc build updates (#17)" This reverts commit e9b5810. * Undo commits * Revert "Undo commits" This reverts commit 147ba2a. * Add test files * Switch to using np.testing.assert_allclose * Add test for sloped field layout * Move test_twoaxistracker to twoaxistracker PR * Add conftest.py to avoid duplicate fixtures * Remove imports of fixtures from conftest * Remove unnecessary imports * Increase test tolerance for test_field_slope * Add multi-polygon active area test * Add empty __init__.py * Use "from .conftest" as Kevin said.. * Base square_field_layout_slope fixture off square_field_layout * Fix linter error * Remove backslash in conftest.py Co-authored-by: Kevin Anderson <[email protected]> * Add get_max_shading_elevation func Also, correct the relative slope calculation * Correct relative_slope in conftest * Add max elevation to TwoAxisTrackerField * Update _field_layout_plot function * Make get_horizon_elevation_angle separate function * Add documentation entries * Add test coverage * Fix linter * Minor documentation updates * Update twoaxistracking_logo.svg Include the shading from the base of the reference collector. * Update twoaxistracking/layout.py Co-authored-by: Kevin Anderson <[email protected]> * Implement changes from review by kanderso-nrel * Fix linter error * Rewrite max_shading_elevation function * Fix linter * Add check for total area enclosing active areas * Updates addressing review by kanderso-nrel Co-authored-by: Kevin Anderson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds four files:
which adds full test coverage of the package.